Skip to content

Conversation

asgerf
Copy link
Contributor

@asgerf asgerf commented Aug 29, 2025

The XUnit family of library models contained an override of getFirstToken and getLastToken. Overriding the common definitions was not actually necessary, and induced some dependencies that are problematic for overlay locality.

This PR removes the problematic dependency by introducing separate root-defs for the custom interpretation of these predicates.

@github-actions github-actions bot added the JS label Aug 29, 2025
@asgerf asgerf added no-change-note-required This PR does not need a change note JS and removed JS labels Aug 29, 2025
@asgerf asgerf marked this pull request as ready for review September 1, 2025 08:18
@asgerf asgerf requested a review from a team as a code owner September 1, 2025 08:18
@Copilot Copilot AI review requested due to automatic review settings September 1, 2025 08:18
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR removes overridden implementations of getFirstToken and getLastToken predicates in the xUnit.qll library to avoid problematic dependencies for overlay locality. The change introduces separate root definitions for custom interpretations of these predicates instead of overriding common definitions.

  • Introduces a FinalExpr type alias to avoid inheritance conflicts
  • Adds explicit getFirstToken and getLastToken methods to BracketedListOfExpressions class
  • Refactors ArrayExprIsABracketedListOfExpressions to use instance-of syntax instead of direct inheritance

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
javascript/ql/lib/semmle/javascript/frameworks/xUnit.qll Refactored inheritance structure and added explicit token accessor methods
javascript/ql/test/library-tests/frameworks/xUnit/tests.expected Updated test expectations to reflect changes in token positioning

@asgerf asgerf merged commit d39263d into github:main Sep 10, 2025
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
JS no-change-note-required This PR does not need a change note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants